Skip to content

feature: Update Image.getSize/getSizeWithHeaders methods to return a promise - #42895

Closed
retyui wants to merge 1 commit into
react:mainfrom
retyui:feature/retyui/get-size-promise-like-api
Closed

feature: Update Image.getSize/getSizeWithHeaders methods to return a promise#42895
retyui wants to merge 1 commit into
react:mainfrom
retyui:feature/retyui/get-size-promise-like-api

Conversation

@retyui

@retyui retyui commented Feb 6, 2024 •
edited
Loading

Copy link
Copy Markdown
Contributor

Summary:

Image.getSize/getSizeWithHeaders are still working in old fashioned "callback" way

Image.getSize(uri, function success(width,height) {  }, function failure(){   } ); // undefined
Image.getSizeWithHeaders(uri, headers, function success(width,height) {  }, function failure(){   } ); // undefined

But in 2024 more developers prefer use async/await syntax for asynchronous operations

So, in this PR I added support for Promise API with backward compatibility, modern way:

Image.getSize(uri).then(({width,height}) => {    }); // Promise
Image.getSizeWithHeaders(uri, headers).then(({width,height}) => {    }); // Promise

Changelog:

[GENERAL] [ADDED] - Image.getSize/getSizeWithHeaders method returns a promise if you don't pass a success callback

Test Plan:

  1. ts: New test cases added in typescript tests
  2. runtime: you can create a new project and put code from this PR into the next files
    a. node_modules/react-native/Libraries/Image/Image.android.js
    b. node_modules/react-native/Libraries/Image/Image.ios.js

@facebook-github-bot facebook-github-bot added CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team. labels Feb 6, 2024
@retyui retyui changed the title feature: Update Image.getSize method to return a promise feature: Update Image.getSize/getSizeWithHeaders methods to return a promise Feb 6, 2024
javache
javache reviewed Feb 6, 2024
View reviewed changes
Comment thread packages/react-native/Libraries/Image/Image.android.js Outdated
@retyui
retyui requested a review from javache February 6, 2024 22:08
@retyui

This comment was marked as outdated.

Sign in to view
@cipolleschi

cipolleschi commented Feb 7, 2024

Copy link
Copy Markdown
Contributor

/rebase - this commet will rebase the PR on top of main automatically.

All these changes are JS changes only, there is no reason why the native builds should fail. Let's see if a rebase fixes CI.

@analysis-bot

analysis-bot commented Feb 7, 2024

Copy link
Copy Markdown
Platform Engine Arch Size (bytes) Diff
android hermes arm64-v8a 17,233,283 -187
android hermes armeabi-v7a n/a --
android hermes x86 n/a --
android hermes x86_64 n/a --
android jsc arm64-v8a 20,600,121 +53
android jsc armeabi-v7a n/a --
android jsc x86 n/a --
android jsc x86_64 n/a --

Base commit: c1b8f37
Branch: main

@facebook-github-bot

facebook-github-bot commented Feb 19, 2024

Copy link
Copy Markdown
Contributor

@cipolleschi has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@retyui

retyui commented Feb 26, 2024 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

the Facebook Internal - Builds & Tests job has failed, do I need to change smth? @cipolleschi

@cipolleschi

cipolleschi commented Feb 26, 2024

Copy link
Copy Markdown
Contributor

I was looking into that. There is an internal E2E test that is failing and I need to understand why. I'll probably look into it tomorrow morning, as the day is almost over and I have another meeting coming up. :(
(it's the only test that is failing, all the other signals are green... ¯_(ツ)_/¯ )

@github-actions

github-actions Bot commented Feb 27, 2024

Copy link
Copy Markdown

This pull request was successfully merged by @retyui in 2c1bcba.

When will my fix make it into a release? | Upcoming Releases

@github-actions github-actions Bot added the Merged This PR has been merged. label Feb 27, 2024
@retyui
retyui deleted the feature/retyui/get-size-promise-like-api branch February 27, 2024 10:54
@retyui retyui mentioned this pull request Mar 1, 2024
Merged
Abbondanzo pushed a commit to react/react-native-website that referenced this pull request Nov 28, 2024
@retyui
Promote to use a Promise api instead of callbacks (#4031)
3cc4d43 
react-native pr: react/react-native#42895
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@cipolleschi cipolleschi cipolleschi approved these changes
@javache javache Awaiting requested review from javache

Assignees

No one assigned

Labels

CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Merged This PR has been merged. Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@retyui @cipolleschi @analysis-bot @facebook-github-bot @javache